Skip to content

Add Psalm static analysis (type checking) with CI and baseline#733

Open
alies-dev wants to merge 4 commits into
code16:9.xfrom
alies-dev:add-psalm-static-analysis
Open

Add Psalm static analysis (type checking) with CI and baseline#733
alies-dev wants to merge 4 commits into
code16:9.xfrom
alies-dev:add-psalm-static-analysis

Conversation

@alies-dev

@alies-dev alies-dev commented Jun 21, 2026

Copy link
Copy Markdown

What this adds

Psalm static type analysis for src/, via psalm-plugin-laravel:

  • Type analysis with a Laravel-aware config (psalm.xml, errorLevel="8").
  • CI workflow (.github/workflows/psalm.yml): one Psalm pass → GitHub annotations + SARIF to Code Scanning, fails on new findings. Actions are SHA-pinned; egress is locked with step-security/harden-runner; actions/checkout runs with persist-credentials: false.
  • Baseline (psalm-baseline.xml): existing findings are suppressed so CI is green from day one and only new issues fail.
  • composer.lock committed: the baseline-gated CI needs a deterministic dependency tree (a floating resolution shifts findings and the gate flaps). Drop it if you'd rather run the Psalm job against a fixed matrix instead.

This is a library, so there are no taint/security findings (user-input entry points live in consuming apps). The value here is type checking, complementary to your test suite.

Fixes already included (45)

Rather than baseline everything, this PR fixes the issues that have a clean resolution:

  • 24× TooFewArguments — typed the from() array parameter with a sealed array{...} shape in 3 Data factories (CommandData, FormSelectFieldData, EntityListConfigData) as a worked example (see below).
  • 11× InvalidParamDefault — nullable promoted params (?array $x = null) had a non-null @var docblock; added |null.
  • MissingTemplateParam — classes implementing Arrayable/ArrayAccess now declare @implements ...<array-key, mixed>.
  • UndefinedDocblockClassFormatsEditorEmbeds typed $field with the trait SharpFieldWithEmbeds (traits aren't types) instead of the interface IsSharpFieldWithEmbeds.
  • UndefinedAttributeClass — imported Spatie\TypeScriptTransformer\Attributes\Optional in FormDateFieldData.

What remains baselined (111) and how to shrink it

count type nature
79 TooFewArguments Psalm can't verify new self(...$array) named-arg unpack (see below)
21 UndefinedDocblockClass a trait used as a type in test helpers
9 UndefinedClass optional deps (pragmarx/google2fa ×4, laravel/octane ×4) + 1 framework-stub edge
2 NoValue config() return-type narrowing false positive

1. TooFewArguments (79) — biggest win

Your Data factories build an array and unpack it as named args:

public static function from(array $config): self
{
    $config = [...$config, 'type' => CommandType::from($config['type']), ...];
    return new self(...$config);   // Psalm: "Too few arguments ... expecting key to be passed"
}

from()'s $config is a bare array, so Psalm can't see the keys and can't prove the required constructor params are supplied. Recommended fix: type the from() parameter with the input contract:

/**
 * @param array{key: string, label: string|null, type: int|string, ...} $config
 */
public static function from(array $config): self

Use the raw input types (what callers pass before the in-body transforms), e.g. type: int|string because the body does CommandType::from($config['type']). This is what the 3 example classes do, and it documents the public contract in one place.

One consequence: with the shape known, the [...$config, 'key' => override] rebuild reads as a duplicate key, so Psalm raises DuplicateArrayKey on each intentionally-overridden key. That override is deliberate here, so this PR demotes it in psalm.xml:

<DuplicateArrayKey errorLevel="info"/>

(Alternative if you don't want a project-wide demote: put the /** @var array{...} $config */ on the rebuilt local instead of the parameter — it annotates the post-merge result and avoids DuplicateArrayKey without a config change, but documents an internal value rather than the public contract.)

Either way, apply the shape to the rest of src/Data/** and the 79 clear. Or, for a fast project-wide pass without per-class shapes, demote the rule: <TooFewArguments errorLevel="info"/> (drops all 79 from the gate; baseline → ~32).

2. UndefinedDocblockClass (21) — extract an interface

All 21 are in src/Utils/Testing/** (Pending* helpers) with @var TestCase&SharpAssertions $test. SharpAssertions is a trait; a trait can't be a type. Extract an interface declaring the trait's public assertion methods, reference it in the docblocks instead.

3. The rest (11)

  • UndefinedClass optional deps: add pragmarx/google2fa / laravel/octane to require-dev (or suggest) to analyze them, otherwise leave baselined.
  • NoValue: false positives; leave baselined or @psalm-suppress at the line with a one-word reason.

Prompts to fix the top clusters

Copy-paste for an AI assistant (or as a checklist) against this checkout:

TooFewArguments (parameter shapes):

For every src/Data/** class with a static factory from(array $x) that ends in return new self(...$x), add a /** @param array{...} $x */ to the method, where the shape lists each constructor parameter as a key (required as name: type, defaulted as name?: type). Use the raw input types — for any key the body transforms before the spread (e.g. 'type' => SomeEnum::from($x['type'])), use the pre-transform type (int|string), not the constructor's type. Do not change runtime behavior. After each file run vendor/bin/psalm --no-cache <file> and confirm zero TooFewArguments, then vendor/bin/pint <file>. DuplicateArrayKey on the overridden keys is expected and already demoted to info in psalm.xml.

UndefinedDocblockClass (trait-as-type):

SharpAssertions is a trait used as a type in src/Utils/Testing/** docblocks (@var TestCase&SharpAssertions). Extract an interface declaring every public method of the SharpAssertions trait, make the consumers implement it, and replace SharpAssertions with that interface in all intersection types. Run Psalm after and confirm the UndefinedDocblockClass findings in Utils/Testing are gone.

After shrinking, regenerate the baseline so it only holds what truly remains:

vendor/bin/psalm --set-baseline=psalm-baseline.xml --no-cache

Docs: https://github.com/psalm/psalm-plugin-laravel

Alies Lapatsin added 4 commits June 21, 2026 23:12
Adds psalm/plugin-laravel with a Laravel-tuned psalm.xml, a single-pass
Psalm CI workflow (SARIF to Code Scanning), and a baseline so CI is green
from day one. errorLevel 8 keeps the baseline reviewable; lower it over time.

Fixes a real docblock bug surfaced by the analysis: FormatsEditorEmbeds typed
$field with the trait SharpFieldWithEmbeds (traits are not types) instead of the
interface IsSharpFieldWithEmbeds.

Commits composer.lock (normally gitignored for a package) so the baseline-gated
Psalm CI resolves the same dependency tree the baseline was generated against.
Three Data classes now annotate the spread source with a sealed array{...}
shape before `new self(...$array)`, so Psalm can verify the named-argument
unpack and stops reporting TooFewArguments (24 fewer findings).

This is the recommended fix maintainers can replicate across src/Data/**:
give the array the constructor's shape instead of a bare `array`. Classes done:
CommandData, FormSelectFieldData, EntityListConfigData.
…ute import

- InvalidParamDefault (11): nullable promoted params (?array $x = null) had a
  non-null @var docblock; added |null so the docblock matches the default.
- MissingTemplateParam (9): classes implementing Arrayable/ArrayAccess now
  declare @implements ...<array-key, mixed>.
- UndefinedAttributeClass (1): import Spatie\...\Attributes\Optional in
  FormDateFieldData (the #[Optional] attribute was unresolved).

Baseline 134 -> 113. Remaining baselined findings are not safe one-line fixes:
trait-used-as-type in test helpers (needs an extracted interface), optional-dep
references (google2fa, octane), and a couple of narrowing/override false positives.
Switch the worked-example Data factories from a @var on the rebuilt local to a
@param array{...} on from(), documenting the public input contract instead of an
internal annotation. The spread-with-override rebuild then trips DuplicateArrayKey
on the overridden keys, which is intentional here, so DuplicateArrayKey is demoted
to info in psalm.xml.

This is the recommended pattern for the remaining src/Data/** factories:
type from()'s array parameter, not the local. Baseline 113 -> 111.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant